Cross-building with Scala 3 by benderpremier · Pull Request #443 · xebia-functional/fetch

benderpremier · 2021-02-19T15:29:11Z

Closes #439

Draft PR to cross build for scala 3

Adapt the build for scala 3
Use type pattern : instead of extractor @
Update example dependencies

Caveats:

fetchJS tests do not compile for scala 3 as scalatest isn't published for scalaJS with scala 3 ( https://mvnrepository.com/artifact/org.scalatest/scalatest )
fetch-example http4s isn't published for scala 3 yet.
fetch-example monix isn't published for scala 3 yet.
sbt-mdoc-toc sbt plugin isn't published for scala 3 yet. I think sbt-mdoc-toc transitively depends on mdoc being ready for scala3. According to this issue, it is currently published for scala3 but only to allow worksheet support
sbt-scoverage isn't published for scala 3 yet. (See this issue

…c for

… argument

franciscodr · 2021-02-24T08:20:48Z

  .settings(commonCrossDependencies)
+  .settings(
+    scalaVersion := "3.0.0-M3",
+    crossScalaVersions := Seq("2.12.11", "2.13.2", "3.0.0-M3")


I guess we could use the latest versions for Scala 2.12 and 2.13:

2.12.13

2.13.5

benderpremier · 2021-02-25T17:45:43Z

I've updated the 2 scala versions. I've tried to find workarounds to get the build to pass, but haven't settled on anything satisfactory. It looks like at this stage we still have too many dependencies that aren't published for scala 3.

juanpedromoreno · 2021-02-25T17:57:50Z

Hi @benderpremier , thanks for helping with the Scala 3 migration! Some comments:

I think scalatest is cross-compiled with [scala 3-RC1]
- https://github.com/scalatest/scalatest/blob/d7b65ba585106ba655a856601d5e43cea965ab25/project/DottyBuild.scala
- https://github.com/scalatest/scalatest/releases/tag/release-3.2.4
sbt-mdoc-toc: I don't think we are using this plugin?
Examples: as we are not publishing them in maven central, maybe we could avoid cross-compilation for example modules. Makes sense?
sbt-scoverage: this one might be tricky. Perhaps, we could disable code-coverage for now.

Again, thanks!

benderpremier · 2021-02-25T21:05:04Z

Thanks for the feedback @juanpedromoreno.

I think scalatest is cross-compiled with [scala 3-RC1]

It is cross compiled for scala 3 on the JVM, but not for scala 3 on scalaJS. For the moment I've disabled cross compilation on scalaJS with.

lazy val fetchJS = fetch.js
  .disablePlugins(ScoverageSbtPlugin)
  .settings(crossScalaVersions := scala2Versions)

sbt-mdoc-toc: I don't think we are using this plugin?

I think we do here (on line 9). It generates the table of content that we see in the readme.md at the root of the repo. However, I think we can scope the mdoc task to only run on 2.13

Examples: as we are not publishing them in maven central, maybe we could avoid cross-compilation for example modules. Makes sense?

Yes I think it makes sense. I'll only compile this project with 2.13.

sbt-scoverage: this one might be tricky. Perhaps, we could disable code-coverage for now.

If everyone is happy with that let's do it 💪.

benderpremier · 2021-03-02T12:29:06Z

I've removed the code coverage plugins and adapted the build. I think if we want to publish for 3.0.0-RC1 that could do the trick.

rolandtritsch · 2021-03-22T07:11:22Z

@benderpremier @franciscodr Some Scala3 porting projects will trigger some (significant) code-refactoring. It would be good to have this. Can we merge this and publish a new version (1.4.3)? Question: Am I right to assume that sbt-scoverage does not need to be updated, but that I need to set coverageScalacPluginVersion to 1.4.3 to make this work?

cb372 · 2021-03-22T10:40:34Z

 # Fetch

-[![Join the chat at https://gitter.im/47deg/fetch](https://badges.gitter.im/47deg/fetch.svg)](https://gitter.im/47deg/fetch?utm_source=badge&utm_medium=badge&utm_campaign=pr-badge&utm_content=badge) [![codecov.io](http://codecov.io/github/47deg/fetch/coverage.svg?branch=master)](http://codecov.io/github/47deg/fetch?branch=master) [![Maven Central](https://img.shields.io/badge/maven%20central-1.2.1-green.svg)](https://oss.sonatype.org/#nexus-search;gav~com.47deg~fetch*) [![License](https://img.shields.io/badge/license-Apache%202-blue.svg)](https://raw.githubusercontent.com/47deg/fetch/master/LICENSE) [![Latest version](https://img.shields.io/badge/fetch-1.2.1-green.svg)](https://index.scala-lang.org/47deg/fetch) [![Scala.js](http://scala-js.org/assets/badges/scalajs-0.6.15.svg)](http://scala-js.org) [![GitHub Issues](https://img.shields.io/github/issues/47deg/fetch.svg)](https://github.com/47deg/fetch/issues)
+[![Join the chat at https://gitter.im/47deg/fetch](https://badges.gitter.im/47deg/fetch.svg)](https://gitter.im/47deg/fetch?utm_source=badge&utm_medium=badge&utm_campaign=pr-badge&utm_content=badge) (http://codecov.io/github/47deg/fetch?branch=master) [![Maven Central](https://img.shields.io/badge/maven%20central-1.2.1-green.svg)](https://oss.sonatype.org/#nexus-search;gav~com.47deg~fetch*) [![License](https://img.shields.io/badge/license-Apache%202-blue.svg)](https://raw.githubusercontent.com/47deg/fetch/master/LICENSE) [![Latest version](https://img.shields.io/badge/fetch-1.2.1-green.svg)](https://index.scala-lang.org/47deg/fetch) [![Scala.js](http://scala-js.org/assets/badges/scalajs-0.6.15.svg)](http://scala-js.org) [![GitHub Issues](https://img.shields.io/github/issues/47deg/fetch.svg)](https://github.com/47deg/fetch/issues)


You removed half of the codecov badge but left (http://codecov.io/github/47deg/fetch?branch=master) - is that intentional?

That wasn't intentional. I'll fix it 👍 .

cb372 · 2021-03-22T10:51:50Z

-  }
-
-}
+///*


What's the plan for this file? Should we delete it instead of commenting it out?

I have uncommented all examples and only cross-compile them for scala2 versions. I assume we can wait until all libraries we depend on in the examples are published for scala3 to cross-compile the examples to scala3 as well.

cb372 · 2021-03-22T11:23:09Z

  ): BlockedRequest[F] =
    (x.request, y.request) match {
-      case (a @ FetchOne(aId, ds), b @ FetchOne(anotherId, _)) =>
+      case (a: FetchOne[Any, Any], b: FetchOne[Any, Any]) =>


This is a bit unfortunate. I can't claim to fully understand all this code, but as far as I can tell, it was taking advantage of bugs in the Scala compiler's GADT support in order to compile?

I had a quick look but I have a feeling that any attempt to address this properly would become a rewrite of most of the library. We could at least try to suppress the erasure warnings?

Something we can try if the @ operator doesn't work is to rewrap the pattern again if needed?

case (FetchOne(aId, ds), FetchOne(anotherId, ds2)) => val a = FetchOne(aId, ds) val b = FetchOne(anotherId, ds2)

I tried that but I am getting the same compilation error:

[error] -- [E007] Type Mismatch Error: /Users/damien/code/fetch/fetch/src/main/scala/fetch.scala:167:48 [error] 167 | val combined = combineIdentities(a, b) [error] | ^ [error] | Found: (b : fetch.package.FetchOne[I$2, A$2]) [error] | Required: fetch.FetchQuery[I$1, A$1]

The compiler wants to prove that the type parameters of FetchOne a and b are the same but it can't. We lose those parameters when we use FetchRequest.

I'll try to see if we can keep track of the I and A type params along the way. I gave it a quick go now and it doesn't seem so straightforward.

I spent some time this afternoon trying to keep track of the I and avoid the FetchOne[Any, Any]. However, I end up bubbling that type parameter all the way up in the API (in the pure and apply method of the Fetch object) which I don't think we want to do.

I am not sure there is a way around using Any here. If you look at how the Fetch are created, we widen to Any in the fetch constructor methods (apply, optional) and then cast with asInstanceOf[A]. Looks like this is where we loose the I we would need here.

Also the monad instance for fetch, implicit def fetchM[F[_]: Monad]: Monad[Fetch[F, *]] would have to be reworked if we push the I all the way up which seems complicated.

pepegar

This looks great Damian,

My only concern is the pattern matches in which we're matching patterns using the pat: Type form instead of capturing the match in a varialbe with v @ Type, we loose type safety that way.

Also, for HTTP4s, I think you can use one of the milestonre releases like 1.0.0-M1 (https://mvnrepository.com/artifact/org.http4s/http4s-dsl_2.13/1.0.0-M19) that crossbuilds for scala 3.0.0-RC1

rolandtritsch · 2021-03-23T06:48:30Z

Thanks for all the reviews!!! (cc: @benderpremier @franciscodr)

rolandtritsch · 2021-03-26T16:44:08Z

@benderpremier I am interested to get this merged/published. Let me know, if/how I can help.

benderpremier · 2021-03-27T13:36:09Z

I am not sure how we want to move forward with this. I agree that having the a: FetchOne[Any, Any], isn't great. As Chris pointed out, looks like we were taking advantage of a limitation of the scala2 compiler. I guess our choice are:

merge with those Any which seems to be on par with what the library is doing now
fix this before publishing for scala3

juanpedromoreno

I'd suggest opening an issue for the FetchOne[Any, Any] improvement, and try to move this forward.

LGTM, thanks @benderpremier !

The base branch was changed.

rolandtritsch · 2021-04-02T09:21:23Z

@benderpremier I agree with @juanpedromoreno: Would suggest to merge now and create a followup ticket to work on FetchOne. WDYT?

benderpremier · 2021-04-02T18:07:42Z

@juanpedromoreno I had to update to scala3 RC2. After merging master into this branch, scalatest got updated to 3.2.7 which is only published for RC2. Could you give it a ✅ ?

benderpremier added 6 commits February 19, 2021 13:37

adapt the build to crosscompile to scala3

6f36a95

explicitely use Any and adapt for comprehension without better monadi…

cd951f6

…c for

Syntax Migration Warning method unsafeToFuture must be called with ()…

83fde52

… argument

scala 3 on fetch-debug

be4763c

change clashing symbol :: to :-:

cf80aa6

adapt fetch-example for available libraries

b79c92c

benderpremier force-pushed the ISSUE-439-build-with-scala3 branch from 82dd4ca to cdf6ef0 Compare February 22, 2021 11:24

scalafmt

9308a4a

benderpremier force-pushed the ISSUE-439-build-with-scala3 branch from cdf6ef0 to 9308a4a Compare February 22, 2021 11:24

franciscodr reviewed Feb 24, 2021

View reviewed changes

benderpremier added 2 commits February 25, 2021 11:35

move versions

d3e1c45

deactivate example while waiting for deps to be published

0b723c4

benderpremier force-pushed the ISSUE-439-build-with-scala3 branch from 33f2d0a to 0b723c4 Compare February 25, 2021 17:19

benderpremier mentioned this pull request Feb 25, 2021

Scala3 migration #439

Closed

use 2.13 as base version

c2fcfa9

remove global cross versions

badd0c9

benderpremier self-assigned this Mar 2, 2021

Damien Favre added 2 commits March 2, 2021 12:13

remove code coverage

bdbdd34

test cross versions

af457c8

benderpremier changed the title ~~[WIP] Issue 439 build with scala3~~ Issue 439 build with scala3 Mar 2, 2021

benderpremier marked this pull request as ready for review March 2, 2021 12:27

benderpremier requested a review from franciscodr March 2, 2021 12:29

rolandtritsch mentioned this pull request Mar 22, 2021

Any plans for a Dotty/Scala3 version of scoverage? scoverage/scalac-scoverage-plugin#299

Closed

cb372 reviewed Mar 22, 2021

View reviewed changes

pepegar reviewed Mar 22, 2021

View reviewed changes

remove remnant url

692541d

uncomment the tests

ad6708a

juanpedromoreno previously approved these changes Mar 31, 2021

View reviewed changes

juanpedromoreno changed the base branch from scala3 to master March 31, 2021 08:34

Merge branch 'master' into ISSUE-439-build-with-scala3

ffa1c5a

juanpedromoreno changed the title ~~Issue 439 build with scala3~~ Cross-building with Scala 3 Mar 31, 2021

juanpedromoreno added the enhancement New feature or request label Mar 31, 2021

juanpedromoreno previously approved these changes Mar 31, 2021

View reviewed changes

merge master

2abeab0

benderpremier dismissed juanpedromoreno’s stale review via 2abeab0 April 2, 2021 17:45

update to RC2

13942f1

cb372 approved these changes Apr 2, 2021

View reviewed changes

benderpremier merged commit b16b0df into master Apr 3, 2021

benderpremier deleted the ISSUE-439-build-with-scala3 branch April 3, 2021 09:55

-                }
-              }
+              ///*

Uh oh!

Conversation

benderpremier commented Feb 19, 2021 • edited by juanpedromoreno Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Caveats:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

benderpremier commented Feb 25, 2021

Uh oh!

juanpedromoreno commented Feb 25, 2021

Uh oh!

benderpremier commented Feb 25, 2021

Uh oh!

benderpremier commented Mar 2, 2021

Uh oh!

rolandtritsch commented Mar 22, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pepegar left a comment

Choose a reason for hiding this comment

Uh oh!

rolandtritsch commented Mar 23, 2021

Uh oh!

rolandtritsch commented Mar 26, 2021

Uh oh!

benderpremier commented Mar 27, 2021

Uh oh!

juanpedromoreno left a comment

Choose a reason for hiding this comment

Uh oh!

rolandtritsch commented Apr 2, 2021

Uh oh!

benderpremier commented Apr 2, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

benderpremier commented Feb 19, 2021 •

edited by juanpedromoreno

Loading